Skip to content

Fix Signature to check context before converting $ to dot#5064

Merged
jjohnstn merged 1 commit into
eclipse-jdt:masterfrom
jjohnstn:dollarfix
May 14, 2026
Merged

Fix Signature to check context before converting $ to dot#5064
jjohnstn merged 1 commit into
eclipse-jdt:masterfrom
jjohnstn:dollarfix

Conversation

@jjohnstn
Copy link
Copy Markdown
Contributor

What it does

See referenced issue.

How to test

See referenced issue.

Author checklist

- add a check in Signature.appendClassTypeSignature to see if a
  $ is preceded or followed by a dot before assuming it is an inner
  class and not part of a package name
- add new test to SignatureTests
- for eclipse-jdt#5056
@jjohnstn jjohnstn self-assigned this May 13, 2026
@jjohnstn jjohnstn added the bug Something isn't working label May 13, 2026
@jjohnstn jjohnstn added this to the 4.40 M3 milestone May 13, 2026
@jjohnstn jjohnstn merged commit f10a4e6 into eclipse-jdt:master May 14, 2026
13 checks passed
@jjohnstn jjohnstn deleted the dollarfix branch May 14, 2026 14:42
@cdietrich
Copy link
Copy Markdown

might this have been causing eclipse-xtext/xtext#3708

@cdietrich
Copy link
Copy Markdown

@jjohnstn this seems to break a typename like

org.eclipse.xtext.common.types.testSetups.$StartsWithDollar

package org.eclipse.xtext.common.types.testSetups;

public class $StartsWithDollar {

	public static class Builder {}
	
}

cc @iloveeclipse

@iloveeclipse
Copy link
Copy Markdown
Member

@cdietrich : looking at eclipse-xtext/xtext#3708, I don't understand if this is something which affects the internal Xtext handling of classes with dollar?

The given code name seem to be properly represented in JDT AST and editor/compiler show no problems too:

image

@iloveeclipse
Copy link
Copy Markdown
Member

Also bytecode shows nothing unexpected:
image

@jjohnstn
Copy link
Copy Markdown
Contributor Author

@jjohnstn this seems to break a typename like

org.eclipse.xtext.common.types.testSetups.$StartsWithDollar

package org.eclipse.xtext.common.types.testSetups;

public class $StartsWithDollar {

	public static class Builder {}
	
}

cc @iloveeclipse

I'll take a look.

@jjohnstn
Copy link
Copy Markdown
Contributor Author

@iloveeclipse I have posted: #5073 This fixes the new problems reported in Xtext and still fixes the original issue whereby importing from a package with dollar in the name wasn't working. I will request a review from you there.

@iloveeclipse
Copy link
Copy Markdown
Member

This fixes the new problems reported in Xtext and still fixes the original issue whereby importing from a package with dollar in the name wasn't working

Could you explain which exact poblem Xtext had with recent changes?

@cdietrich
Copy link
Copy Markdown

@iloveeclipse please check the failing https://github.com/eclipse-xtext/xtext/pull/3710/changes
eclipse-xtext/xtext#3708 has some code pointers

@cdietrich
Copy link
Copy Markdown

@szarnekow you might be able to explain more on the details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants